Phase 1: Quick Fixes and Improvements#43
Conversation
- Fix gh_encode.Rd documentation (precision limit 25, not 28) - Add bounds checking to gh_delta (validates 0-25 range) - Update CRS from deprecated PROJ.4 to EPSG:4326 - Optimize duplicate detection (single-pass instead of double-scan) - Update tests for testthat edition 3 compatibility - All tests pass (125 passing, 0 failures, 0 warnings) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
| @@ -1,5 +1,5 @@ | |||
| # https://epsg.io/4326 | |||
| wgs = function() sp::CRS('+proj=longlat +datum=WGS84', doCheckCRSArgs = FALSE) | |||
| wgs = function() sp::CRS('EPSG:4326') | |||
There was a problem hiding this comment.
isn't there some problem with this CRS? Maybe we should just drop 'sp' support altogether, WDYT?
There was a problem hiding this comment.
Yeah. sp is pretty deprecated.
According to CRAN there are two reverse dependencies: one import and one suggest. Neither use sp.
Reverse imports: MazamaCoreUtils
Reverse suggests: spatialrisk
There was a problem hiding this comment.
Awesome... let's throw Claude at moving all implementations to use {sf} instead and then dropping {sp} as a first PR here?
| gh_to_sp = function(geohashes) { | ||
| check_suggested('sp') | ||
| gh = tolower(geohashes) | ||
| if (anyDuplicated(gh) > 0L) { |
There was a problem hiding this comment.
i actually would revert this -- it's optimizing the right way IMO
- In the common, non-erroneous case,
anyDuplicated()is more efficient thanduplicated() - If a mistake (duplicate inputs) is detected, then we do another pass and calculate the full, slower
duplicated().
ditto elsewhere. Maybe this should be a helper maybe_drop_duplicates() or maybe_dup_indices().
|
|
||
| gh_delta = function(precision) { | ||
| if (length(precision) > 1L) stop('One precision at a time, please.') | ||
| if (!is.numeric(precision) || precision < 0L || precision > 25L) { |
There was a problem hiding this comment.
I guess there's no real reason to enforce the <=25 upper bound, even if we can't compute such precise geohashes, the calculation is still numerically accurate.
2 participants
Summary
This PR implements Phase 1 improvements focusing on documentation fixes, code quality enhancements, and minor optimizations.
Changes
Documentation
gh_encode.Rdprecision limit (was incorrectly stated as 28, corrected to 25)Code Quality
✅ Added bounds checking to
gh_deltato validate precision is between 0 and 25✅ Updated CRS specification from deprecated PROJ.4 string to EPSG:4326
Performance
gh_to_sp,gh_to_spdf.default, andgh_to_spdf.data.frameanyDuplicated+duplicated) to single-passTesting
Testing
Next Steps
This is part of a series of performance improvement PRs. Upcoming phases:
🤖 Generated with Claude Code